chore: align with CI workflows of other interface packages #1
Conversation
📝 WalkthroughWalkthroughThe pull request introduces significant changes to the GitHub Actions workflows for the project. A new Changes
Sequence DiagramsequenceDiagram
participant PR as Pull Request
participant Workflow as Conventional PRs Workflow
participant Validator as Semantic PR Validator
PR->>Workflow: Trigger on PR events
Workflow->>Validator: Check PR title
alt Valid Semantic Title
Validator-->>Workflow: Validation Passed
else Invalid Title
Validator-->>Workflow: Validation Failed
Workflow-->>PR: Request Title Correction
end
sequenceDiagram
participant Repo as Repository
participant ReleasePlease as Release Please Workflow
participant PyPI as PyPI Registry
participant GitHub as GitHub Releases
Repo->>ReleasePlease: Push to main branch
ReleasePlease->>ReleasePlease: Create Release
ReleasePlease->>PyPI: Publish Package
ReleasePlease->>GitHub: Create GitHub Release
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/test.yml (2)
7-8: Remove unnecessarybranches-ignoreconfiguration.The empty
branches-ignorearray doesn't serve any purpose and can be removed.pull_request: - branches-ignore: []🧰 Tools
🪛 actionlint (1.7.4)
8-8: "branches-ignore" section should not be empty
(syntax-check)
91-91: Consider using a more flexible test discovery pattern.The current pytest command uses a hardcoded test file path. Consider using a more flexible pattern to automatically discover all test files:
- run: poetry run coverage run -m pytest tests/tests.py + run: poetry run coverage run -m pytest tests/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/conventional-prs.yml(1 hunks).github/workflows/release-please.yml(1 hunks).github/workflows/release.yml(0 hunks).github/workflows/test.yml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/release.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-please.yml
25-25: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
27-27: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
37-37: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test.yml
8-8: "branches-ignore" section should not be empty
(syntax-check)
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
17-17: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
27-27: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
42-42: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
44-44: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
54-54: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
68-68: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
70-70: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
80-80: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/test.yml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/conventional-prs.yml (1)
1-16: LGTM! Good security practices withpull_request_target.The workflow is well-configured for semantic PR title validation. Using
pull_request_targetis appropriate here since the workflow only uses a trusted action and doesn't check out or run PR code.
.github/workflows/release-please.yml
Outdated
| needs: release-please | ||
| if: ${{ needs.release-please.outputs.release_created }} | ||
| steps: | ||
| - uses: actions/checkout@v3 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update GitHub Actions to their latest versions.
Several actions are using outdated versions:
actions/checkout@v3->actions/checkout@v4actions/setup-python@v2->actions/setup-python@v5actions/setup-python@v4->actions/setup-python@v5
Also applies to: 27-27, 37-37
🧰 Tools
🪛 actionlint (1.7.4)
25-25: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/release-please.yml
Outdated
| - uses: actions/setup-python@v2 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Install poetry | ||
| run: pip install poetry | ||
|
|
||
| - name: Determine dependencies | ||
| run: poetry lock | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: poetry | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate Python setup steps.
The Python setup step is duplicated. The first setup can be removed since the second one includes caching.
- - uses: actions/setup-python@v2
- with:
- python-version: "3.11"
-
- name: Install poetry
run: pip install poetry
- name: Determine dependencies
run: poetry lock
- uses: actions/setup-python@v4
with:
python-version: "3.11"
cache: poetry📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/setup-python@v2 | |
| with: | |
| python-version: "3.11" | |
| - name: Install poetry | |
| run: pip install poetry | |
| - name: Determine dependencies | |
| run: poetry lock | |
| - uses: actions/setup-python@v4 | |
| with: | |
| python-version: "3.11" | |
| cache: poetry | |
| - name: Install poetry | |
| run: pip install poetry | |
| - name: Determine dependencies | |
| run: poetry lock | |
| - uses: actions/setup-python@v4 | |
| with: | |
| python-version: "3.11" | |
| cache: poetry |
🧰 Tools
🪛 actionlint (1.7.4)
27-27: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
37-37: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test.yml
Outdated
| formatting: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Check out the code | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Install poetry | ||
| run: pip install poetry | ||
|
|
||
| - name: Determine dependencies | ||
| run: poetry lock | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: poetry | ||
|
|
||
| - name: Install Dependencies using Poetry | ||
| run: poetry install | ||
|
|
||
| - name: Check formatting | ||
| run: poetry run black --check . | ||
|
|
||
| linting: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Check out the code | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Install poetry | ||
| run: pip install poetry | ||
|
|
||
| - name: Determine dependencies | ||
| run: poetry lock | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: poetry | ||
|
|
||
| - name: Install Dependencies using Poetry | ||
| run: poetry install | ||
|
|
||
| - name: Check code | ||
| run: poetry run flake8 | ||
|
|
||
| testing: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Install poetry | ||
| run: pip install poetry | ||
|
|
||
| - name: Determine dependencies | ||
| run: poetry lock | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: poetry | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| pip install connection-pool # because it is incompatible with poetry | ||
| poetry install | ||
|
|
||
| - name: Run pytest | ||
| run: poetry run coverage run -m pytest tests/tests.py | ||
|
|
||
| - name: Run Coverage | ||
| run: poetry run coverage report -m |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor workflow to reduce duplication using reusable workflows.
Each job (formatting, linting, testing) has identical setup steps. Consider extracting these common steps into a reusable workflow to improve maintainability.
Here's how you could refactor this:
- Create a new file
.github/workflows/setup-poetry.yml:
name: Setup Poetry
on:
workflow_call:
jobs:
setup:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: "3.11"
cache: poetry
- name: Install poetry
run: pip install poetry
- name: Determine dependencies
run: poetry lock
- name: Install Dependencies
run: poetry install- Update test.yml to use the reusable workflow:
name: CI
on:
push:
branches:
- main
pull_request:
jobs:
setup:
uses: ./.github/workflows/setup-poetry.yml
formatting:
needs: setup
runs-on: ubuntu-latest
steps:
- name: Check formatting
run: poetry run black --check .
linting:
needs: setup
runs-on: ubuntu-latest
steps:
- name: Check code
run: poetry run flake8
testing:
needs: setup
runs-on: ubuntu-latest
steps:
- name: Install connection-pool
run: pip install connection-pool
- name: Run pytest
run: poetry run coverage run -m pytest tests/tests.py
- name: Run Coverage
run: poetry run coverage report -m🧰 Tools
🪛 actionlint (1.7.4)
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
17-17: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
27-27: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
42-42: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
44-44: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
54-54: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
68-68: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
70-70: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
80-80: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
.github/workflows/test.yml
Outdated
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Check out the code | ||
| uses: actions/checkout@v3 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update GitHub Actions to their latest versions.
Several actions are using outdated versions:
actions/checkout@v3->actions/checkout@v4actions/setup-python@v4->actions/setup-python@v5
Also applies to: 42-42, 68-68, 17-17, 27-27, 44-44, 54-54, 70-70, 80-80
🧰 Tools
🪛 actionlint (1.7.4)
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/snakemake_interface_logger_plugins/registry/__init__.py (1)
Line range hint
8-8: Remove unused import.The
Listtype fromtypingis imported but never used.-from typing import List, Mapping +from typing import Mapping
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
11-54: 🛠️ Refactor suggestionRefactor workflow to reduce duplication.
The jobs have identical setup steps. Consider extracting these common steps into a reusable workflow.
This is a duplicate of a previous review comment. The suggestion to create a reusable workflow remains valid.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
🧹 Nitpick comments (2)
.github/workflows/release-please.yml (1)
34-39: Add error handling for build steps.Consider adding error handling and validation steps:
- Verify the build artifacts after creation
- Add timeouts to prevent hanging builds
- name: Install the project run: uv sync --all-extras --dev + timeout-minutes: 10 - name: Build source and wheel distribution run: | uv build + # Verify the build artifacts + ls dist/*.whl dist/*.tar.gz + timeout-minutes: 5tests/tests.py (1)
55-92: Consider adding more test scenarios.While the test registry implementation is good, consider adding more test cases:
- Test with invalid log levels
- Test handler creation with various combinations of boolean parameters
Would you like me to generate additional test cases to improve coverage?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pyproject.tomlis excluded by!pyproject.tomluv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/release-please.yml(1 hunks).github/workflows/test.yml(1 hunks)src/snakemake_interface_logger_plugins/registry/__init__.py(2 hunks)tests/tests.py(1 hunks)version.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- version.txt
🧰 Additional context used
📓 Path-based instructions (2)
src/snakemake_interface_logger_plugins/registry/__init__.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
tests/tests.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
🪛 actionlint (1.7.4)
.github/workflows/test.yml
8-8: "branches-ignore" section should not be empty
(syntax-check)
.github/workflows/release-please.yml
24-24: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
42-42: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
56-56: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/test.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
.github/workflows/release-please.yml
[error] 28-28: trailing spaces
(trailing-spaces)
🪛 GitHub Actions: CI
src/snakemake_interface_logger_plugins/registry/__init__.py
[error] 9-9: typing.List imported but unused. Remove unused import: typing.List
🔇 Additional comments (6)
.github/workflows/release-please.yml (2)
24-24: Update actions/checkout to v4.The checkout action is using an outdated version.
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4🧰 Tools
🪛 actionlint (1.7.4)
24-24: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
46-62: LGTM! Secure PyPI publishing configuration.The PyPI publishing job is well configured with:
- Trusted publishing using OpenID Connect
- Proper permissions setup
- Secure artifact handling
🧰 Tools
🪛 actionlint (1.7.4)
56-56: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
src/snakemake_interface_logger_plugins/registry/__init__.py (2)
23-23: LGTM! Architectural change fromLoggerPluginBasetoLogHandlerBase.The change aligns with the PR's objective to update the interface package.
51-51: LGTM! Updated expected attribute type toLogHandlerBase.The change is consistent with the import change and maintains the architectural consistency.
tests/tests.py (2)
16-28: LGTM! Well-structured mock settings class.The
MockSettingsclass provides a good example of how to implement logger plugin settings with proper metadata.
30-53: LGTM! Comprehensive mock plugin implementation.The
MockPluginclass correctly implements the required interface with proper default settings handling.
| - uses: GoogleCloudPlatform/release-please-action@v3 | ||
| id: release | ||
| with: | ||
| release-type: simple |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update release-type for Python package.
The release-type should be set to "python" instead of "simple" for Python packages to ensure proper version management and changelog generation.
- release-type: simple
+ release-type: python📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| release-type: simple | |
| release-type: python |
.github/workflows/test.yml
Outdated
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
There was a problem hiding this comment.
Remove trailing spaces.
There are trailing spaces on lines 15, 29, and 43.
- - uses: actions/checkout@v4
-
+ - uses: actions/checkout@v4Also applies to: 29-29, 43-43
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 15-15: trailing spaces
(trailing-spaces)
| branches: | ||
| - main | ||
| pull_request: | ||
| branches-ignore: [] |
There was a problem hiding this comment.
Remove empty branches-ignore section.
The empty branches-ignore section is unnecessary and can be removed.
pull_request:
- branches-ignore: []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| branches-ignore: [] | |
| pull_request: |
🧰 Tools
🪛 actionlint (1.7.4)
8-8: "branches-ignore" section should not be empty
(syntax-check)
|
Hi @johanneskoester, I updated the workflows to use ruff for formatting and linting. Had some trouble with pylyzer so gave up on that for now. Also stuck with uv over poetry. I don't think I realized the other plugins used poetry - happy to migrate this to poetry, though I do prefer uv. I'm unfamiliar with release-please and how exactly it works and the workflow, but it seems handy. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/release-please.yml (2)
44-44: ** Upgrade upload-artifact action version.**Static analysis indicates that
actions/upload-artifact@v3is outdated. It is recommended to upgrade to a later version (e.g.,actions/upload-artifact@v4) for improved compatibility and performance.- - name: Upload artifacts - uses: actions/upload-artifact@v3 + - name: Upload artifacts + uses: actions/upload-artifact@v4🧰 Tools
🪛 actionlint (1.7.4)
44-44: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
58-58: ** Upgrade download-artifact action version.**Similarly,
actions/download-artifact@v3is flagged as outdated. Updating this to a newer version (for example,actions/download-artifact@v4) can help ensure smoother downstream operations.- - name: Download artifacts - uses: actions/download-artifact@v3 + - name: Download artifacts + uses: actions/download-artifact@v4🧰 Tools
🪛 actionlint (1.7.4)
58-58: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test.yml (1)
11-31: ** Consider consolidating common CI setup steps.**The
formatting,linting, andtestingjobs all include similar steps (checking out code, setting up Python, installing Poetry, and installing dependencies). Refactoring these into a reusable workflow or a common setup script could simplify maintenance and reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
poetry.lockis excluded by!**/*.lockpyproject.tomlis excluded by!pyproject.toml
📒 Files selected for processing (3)
.github/workflows/release-please.yml(1 hunks).github/workflows/test.yml(1 hunks)src/snakemake_interface_logger_plugins/registry/__init__.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/snakemake_interface_logger_plugins/registry/init.py
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-please.yml
44-44: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
58-58: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test.yml
8-8: "branches-ignore" section should not be empty
(syntax-check)
🔇 Additional comments (2)
.github/workflows/release-please.yml (1)
17-17: ** Updaterelease-typefor proper release metadata.**The
release-typeshould be set to"python"instead of"simple"to ensure that version management (including changelog generation) works correctly for Python packages. This comment duplicates previous suggestions..github/workflows/test.yml (1)
8-8: 🛠️ Refactor suggestion** Remove unnecessary empty branches-ignore.**
The
branches-ignore: []field under thepull_requesttrigger is empty and adds unnecessary clutter. Removing it will improve clarity and avoid potential lint warnings.- pull_request: - branches-ignore: [] + pull_request:Likely invalid or redundant comment.
🧰 Tools
🪛 actionlint (1.7.4)
8-8: "branches-ignore" section should not be empty
(syntax-check)
cademirch
force-pushed
the
chore/ci-workflows
branch
from
1c63a51 to
25ef585
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/test.yml (2)
11-31: Formatting Job: Consider Reusing Common Setup StepsThe "formatting" job correctly checks out the code, sets up Python 3.12, installs Poetry (v2.0.0), installs dependencies, and then runs
ruff format --check. However, these setup steps are repeated in the subsequent jobs. To adhere to the DRY principle, consider extracting these common steps into a reusable workflow (e.g., a shared setup job) and then reference it in each job. This will improve maintainability.
32-52: Linting Job: Duplicate Setup StepsSimilar to the formatting job, the "linting" job duplicates the checkout, Python setup, Poetry installation, and dependency installation steps before running
ruff check. Consolidating these repeated operations into a reusable workflow or shared action could reduce duplication and simplify future maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
poetry.lockis excluded by!**/*.lockpyproject.tomlis excluded by!pyproject.toml
📒 Files selected for processing (3)
.github/workflows/release-please.yml(1 hunks).github/workflows/test.yml(1 hunks)src/snakemake_interface_logger_plugins/registry/__init__.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/snakemake_interface_logger_plugins/registry/init.py
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-please.yml
44-44: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
58-58: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test.yml
8-8: "branches-ignore" section should not be empty
(syntax-check)
🔇 Additional comments (7)
.github/workflows/test.yml (3)
1-2: Workflow Naming and Trigger OverviewThe workflow name is set clearly as "CI," and the push trigger on the "main" branch is correctly configured.
8-8: Remove Empty "branches-ignore" SectionThe
branches-ignore: []field under the pull_request event is empty and unnecessary. Empty configuration properties may lead to confusion and trigger warnings from static analysis tools. Consider removing it entirely (unless you plan to ignore specific branches). For example:- pull_request: - branches-ignore: [] + pull_request:🧰 Tools
🪛 actionlint (1.7.4)
8-8: "branches-ignore" section should not be empty
(syntax-check)
53-75: Testing Job: Setup Duplication and Potential Caching ImprovementsThe "testing" job is well-configured to run Pytest with coverage reporting. It repeats the same initial setup (checkout, Python, Poetry, and dependency installation) found in the other jobs. Consider the following enhancements:
- Refactor the common setup into a reusable workflow to minimize duplication.
- Evaluate the possibility of caching Poetry dependencies or the virtual environment to improve CI execution time.
Overall, the testing configuration is correct, but these adjustments might help streamline and speed up the workflow.
.github/workflows/release-please.yml (4)
1-5: Workflow Trigger Configuration is CorrectThe workflow trigger on pushes to the
mainbranch is straightforward and appropriate for automating releases.
6-7: Workflow Name is DescriptiveThe workflow name “release-please” accurately reflects its purpose.
43-47: 🛠️ Refactor suggestionUpdate Artifact Upload Action Version
Static analysis indicates that the runner for
actions/upload-artifact@v3is outdated. Consider updating this action to a newer version (for example,v4if available) to improve compatibility and take advantage of recent improvements.- - name: Upload artifacts - uses: actions/upload-artifact@v3 + - name: Upload artifacts + uses: actions/upload-artifact@v4✅ Verification successful
Artifact Upload Action Version Update Recommendation
The suggested change in the workflow file is valid. Using
actions/upload-artifact@v4can help take advantage of the latest improvements and compatibility fixes. In your file.github/workflows/release-please.yml(lines 43–47), update the version accordingly after reviewing the v4 release notes for any breaking changes.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
🧰 Tools
🪛 actionlint (1.7.4)
44-44: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
57-61: 🛠️ Refactor suggestionUpdate Artifact Download Action Version
Similarly, the static analysis suggests that
actions/download-artifact@v3is outdated. Upgrading to a newer version (for example,v4if available) can help ensure smoother artifact handling.- - name: Download artifacts - uses: actions/download-artifact@v3 + - name: Download artifacts + uses: actions/download-artifact@v4✅ Verification successful
Artifact Download Action Version Update Verified
The latest release for actions/download-artifact is confirmed as v4.1.8, which means updating the GitHub workflow to use
actions/download-artifact@v4is valid and beneficial.
- Update the reference in
.github/workflows/release-please.yml(lines 57-61) accordingly.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash echo "Fetching latest release version for actions/download-artifact from GitHub API..." latest=$(curl -s "https://api.github.com/repos/actions/download-artifact/releases/latest" | jq -r '.tag_name') echo "Latest release tag is: $latest"Length of output: 356
🧰 Tools
🪛 actionlint (1.7.4)
58-58: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| jobs: | ||
| release-please: | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| release_created: ${{ steps.release.outputs.release_created }} | ||
| steps: | ||
| - uses: GoogleCloudPlatform/release-please-action@v3 | ||
| id: release | ||
| with: | ||
| release-type: simple | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Update the release-type Parameter to "python" for Python Packages
The GitHub Actions workflow in .github/workflows/release-please.yml currently sets the parameter as release-type: simple. Since this repository is a Python package (as evidenced by the use of Python setup, Poetry, and PyPI publishing steps), the parameter should be updated to "python" for correct version management, changelog generation, and proper PyPI integration.
- File:
.github/workflows/release-please.yml - Location: Under the
release-pleasejob, change lines whererelease-type: simpleis specified. - Proposed Change:
- release-type: simple + release-type: python
🔗 Analysis chain
Update the Release Type Parameter
The release-please job uses GoogleCloudPlatform/release-please-action@v3 with the parameter release-type set to "simple". For Python packages, this value should be updated to "python" to ensure proper version management, changelog generation, and integration with PyPI publishing.
- release-type: simple
+ release-type: python🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Display the contents of the GitHub Actions workflow file to verify the release-type parameter.
cat .github/workflows/release-please.yml
Length of output: 1525
|
@johanneskoester, I migrated this to poetry. lmk if all looks good re the release workflow then can merge. |
I have simply copied over the CI workflows of the other interface repos. I did not know about pypi trusted publishing though. Feel free to modernize this accordingly by integrating the pypi publishing you had in your previous version. Using release please is very handy though.
Summary by CodeRabbit
New Features
Workflow Changes
Version Update
0.1.1